feat: support OpenAPI 3.2 additional operations#10956
Conversation
wbizmo
left a comment
There was a problem hiding this comment.
Went through this properly, cloned it and actually ran the tests instead of just reading the diff.
Ran the exact test command from the description: 60/60 pass, confirmed it myself. Also ran the full unit suite to check for regressions elsewhere, 841/853 pass, the rest are pre existing skips unrelated to this. Lint is clean too.
Quick heads up on the "14 files changed" count, a couple of those (package lock, CI workflow files) are just leftovers from a master merge, not actually part of this feature.
Two things I'd want to understand better before this merges, not blockers:
-
getOAS3RequiredRequestBodyContentTypeandisMediaTypeSchemaPropertiesEqualnow read fromspecJsonWithResolvedSubtrees(state)instead of the raw resolvedSubtrees branch. That changes behavior for regular standard method operations too, not just the new additionalOperations stuff. Tests still pass so it seems fine, but the checklist says "not breaking changes" and this quietly changes the data source for existing code paths. Was that intentional? -
This depends on an unreleased swagger js PR (#4238) for full support, which is handled the right way (no fork/branch pin). But all the new tests mock the state directly, there's nothing testing this against the actual released swagger-client resolver. Might be worth confirming it behaves once that release is out.
Otherwise this is solid work, good test coverage on the new logic and nothing broke anywhere else.
Thanks for checking this carefully.
|
wbizmo
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround on this, and for actually engaging with both points instead of just closing them out.
The new test on resolved subtree precedence is exactly what I was hoping to see, it sets json and resolvedSubtrees to different values on purpose and asserts the resolved subtree wins for a standard operation. That documents the intended behavior and protects it going forward. Ran the full suite locally, 854 tests, all passing, no regressions.
On the swagger-js dependency, that's a reasonable sequencing plan. Makes sense to land this now with mocked tests and follow up with a real integration check once swagger-client ships OAS 3.2 support.
Approving.
Description
Adds OpenAPI 3.2 Path Item
additionalOperationssupport to Swagger UI. Custom operation methods are discovered and rendered as first-class operations, including their request parameters, request bodies, responses, deep links, validation state, and operation-specific metadata.Custom methods retain the exact method token from the OpenAPI document. Standard methods continue to use the existing built-in behavior, while custom methods receive neutral styling rather than being added as hard-coded method names.
Motivation and Context
OpenAPI 3.2 allows arbitrary operation method names under
additionalOperations. Swagger UI previously omitted those operations from the operation list and could not resolve their operation data through the normal selector paths.This PR depends on the corresponding Swagger Client support in swagger-api/swagger-js#4238. Swagger UI should consume a released Swagger Client version containing that change; this PR intentionally does not add a dependency on a fork, branch, or immutable development tag.
How Has This Been Tested?
npm run lint-errorsnpm run test:unit -- --runInBand test/unit/core/plugins/oas32/query-operation-rendering.test.js test/unit/core/plugins/oas32/spec-extensions/wrap-selectors.test.js test/unit/core/plugins/spec/selectors.jsTypes of changes
src/is unmodified: changes to documentation, CI, metadata, etc.)package.json)My changes...
Documentation
Automated tests